feat(rpc): add rowid-forward message pagination by vincentkoc · Pull Request #201 · openclaw/imsg

vincentkoc · 2026-08-01T00:07:07Z

Summary

add additive messages.after JSON-RPC pagination in stable message.ROWID ASC order
return an authoritative physical next_rowid and database-backed has_more signal
preserve URL-preview coalescing across page boundaries without losing cursor progress
support optional chat filtering, attachments, conversion, and standalone reaction events
advertise and document the capability for downstream consumers

Closes #200.

Downstream consumer: openclaw/openclaw#116961

Contract

since_rowid is required, non-negative, and exclusive
chat_id is optional; omit it to page across all chats
limit defaults to 100 and is capped at 500
results are ordered by physical ROWID, not timestamp
callers persist next_rowid after every response, including empty pages
callers continue only while has_more is true
callers that need reaction rows covered by the cursor set include_reactions: true

Validation

focused rpcMessagesAfter|messagesAfterPage tests: 10 passed
non-stdout-capture suite: 356 tests passed
make lint: passed; 12 existing non-serious warnings, none in changed files
make build: universal CLI/helper build passed; existing unarchiveObjectWithData: deprecation warning only
exact-tree automated review: clean, no actionable findings
git diff --check: passed

A full local make test attempt remains blocked by a pre-existing StdoutCapture pipe deadlock in output-heavy tests such as statusCommandProducesTextOutput and chatBackgroundStatusEmitsReadOnlyJSON. Those 139 capture-based tests were excluded from the broad local run; focused changed-surface tests pass.

clawsweeper · 2026-08-01T00:20:27Z

Codex review: found issues before merge. Reviewed August 2, 2026, 1:34 PM ET / 17:34 UTC.

ClawSweeper review

What this changes

This PR exposes a new messages.after JSON-RPC endpoint that returns Messages records in forward ROWID order with a durable continuation cursor, optional reactions, attachments, and chat filtering.

Merge readiness

⚠️ Needs maintainer review before merge - 5 items remain

Keep open. Current main has internal ROWID-forward scanning but no public messages.after RPC, so this PR remains the active implementation candidate for #200. Before merge, the shared Message documentation must stop saying reaction events are live-watch-only, and the new stable RPC contract needs the API owner's approval. Likely related people: steipete is the high-confidence routing candidate from the existing cursor implementation and current maintainer review.

Priority: P3
Reviewed head: b61a3c95bcc3d326fe09b165e21ca40ca741991b
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The real behavior evidence is strong, but a P2 public-documentation contradiction and a pending API-contract decision prevent merge readiness.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): A collaborator independently recorded focused test results, a successful macOS build, and a live release-RPC fixture run showing a message, `next_rowid: 1`, and `has_more: false`; no additional contributor proof is needed.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): A collaborator independently recorded focused test results, a successful macOS build, and a live release-RPC fixture run showing a message, `next_rowid: 1`, and `has_more: false`; no additional contributor proof is needed.
Evidence reviewed	5 items	Current main lacks the requested RPC surface: Current main lists `messages.history` but neither `messages.after`, its handler, nor its page type appears in the RPC, documentation, or test paths; the existing `messagesAfter` helper is internal-only. Existing cursor behavior predates this PR: The current internal `messagesAfter` helper already scans forward over Message ROWIDs and supports optional reactions, establishing the underlying feature boundary this PR exposes through RPC. Shared Message documentation contradicts the proposed endpoint: The new RPC documentation advertises `include_reactions` and returns `[Message]`, while the current shared Message reference says reaction events only surface in the live watch stream.
Findings	1 actionable finding	[P2] Align the shared reaction-event documentation
Security	None	None.

How this fits together

The iMessage RPC server turns read requests into queries against the local Messages SQLite database and serializes Message payloads for long-running clients. The added endpoint would give downstream catch-up consumers a stable forward cursor instead of relying on newest-first history reads.

flowchart LR
  Client[Catch-up client] --> RPC[JSON-RPC server]
  RPC --> Validation[Request validation]
  Validation --> Store[Messages SQLite store]
  Store --> Paging[ROWID scan and URL preview handling]
  Paging --> Response[Messages plus continuation cursor]

Decision needed

Question	Recommendation
Should `messages.after`, including optional standalone reaction events and physical ROWID cursor semantics, become a stable public RPC contract for downstream catch-up clients?	Approve and document the stable contract: Accept the public endpoint, align the shared Message reaction documentation, and land after the branch is refreshed.

Why: The implementation is additive and proof-backed, but accepting a permanent RPC method and its reaction/cursor behavior is an API-direction decision rather than a mechanical repair.

Before merge

Align the shared reaction-event documentation (P2) - include_reactions makes this endpoint return standalone reaction Message payloads, but docs/json.md still says those events only surface in live watch. Because this result is documented as [Message], update the shared reference before merge so API clients do not receive conflicting payload guidance.
Resolve merge risk (P1) - include_reactions makes standalone reaction events available in a paginated result, but the shared Message reference still tells API users those events only surface in live watch; merging as-is creates contradictory stable API guidance.
Resolve merge risk (P1) - This is an additive public RPC contract with cursor persistence semantics and a downstream consumer, so the API owner must explicitly accept its permanent compatibility boundary before landing.
Resolve merge risk (P1) - GitHub currently reports the draft branch as dirty against main; refresh the branch and re-evaluate the actual merge result after the contract decision.

Findings

[P2] Align the shared reaction-event documentation — docs/rpc.md:96-97

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	742 added, 1 removed across 10 files	The feature spans the public RPC, core cursor logic, documentation, and two focused test files.
Focused regression coverage	10 tests added	Four core pagination tests and six RPC tests exercise cursor ordering, suppressed previews, parameters, and payload behavior.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #200
Summary: This PR is the explicit implementation candidate for the open feature request; it should remain open until the feature contract is approved and the branch can land.

Members:

canonical: feat(rpc): add rowid-forward message pagination #200 - The PR body and linked issue describe the same public messages.after contract, and the issue is still open.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

Approve the contract and synchronize Message docs (recommended)
Make the shared Message reference describe reaction events for both live watch and the new paginated endpoint before merging the approved RPC contract.
Defer reaction pagination
Keep forward message pagination but remove or postpone include_reactions if maintainers do not want this endpoint to extend the stable reaction-event contract.
Pause the endpoint
Close or defer the proposal if a permanent cross-client cursor contract is not wanted in the public RPC surface.

Technical review

Best possible solution:

Approve the messages.after contract only if it is intended as a permanent public RPC, then align docs/json.md and docs/rpc.md so the shared Message reaction semantics cover both watch and messages.after, followed by a refreshed merge check.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR proposes an additive RPC capability rather than repairing an existing failing behavior. The contributor and collaborator nevertheless provide a high-confidence fixture-based after-fix validation path for the proposed behavior.

Is this the best way to solve the issue?

Unclear until an API owner accepts the stable contract. The implementation reuses current-main forward scanning rather than duplicating the database logic, but the shared reaction-event documentation must be corrected before this is the maintainable solution.

Full review comments:

[P2] Align the shared reaction-event documentation — docs/rpc.md:96-97
include_reactions makes this endpoint return standalone reaction Message payloads, but docs/json.md still says those events only surface in live watch. Because this result is documented as [Message], update the shared reference before merge so API clients do not receive conflicting payload guidance.
Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5bdd2b0b003a.

Labels

Label justifications:

P3: This is a feature addition for downstream catch-up ergonomics rather than a current user-facing regression.
merge-risk: 🚨 compatibility: The PR creates a stable public RPC and changes where consumers may receive standalone reaction Message payloads.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): A collaborator independently recorded focused test results, a successful macOS build, and a live release-RPC fixture run showing a message, next_rowid: 1, and has_more: false; no additional contributor proof is needed.
proof: sufficient: Contributor real behavior proof is sufficient. A collaborator independently recorded focused test results, a successful macOS build, and a live release-RPC fixture run showing a message, next_rowid: 1, and has_more: false; no additional contributor proof is needed.

Evidence

What I checked:

Current main lacks the requested RPC surface: Current main lists messages.history but neither messages.after, its handler, nor its page type appears in the RPC, documentation, or test paths; the existing messagesAfter helper is internal-only. (Sources/imsg/RPCServer.swift:36, 5bdd2b0b003a)
Existing cursor behavior predates this PR: The current internal messagesAfter helper already scans forward over Message ROWIDs and supports optional reactions, establishing the underlying feature boundary this PR exposes through RPC. (Sources/IMsgCore/MessageStore+Messages.swift:90, 1d8b679cc3a3)
Shared Message documentation contradicts the proposed endpoint: The new RPC documentation advertises include_reactions and returns [Message], while the current shared Message reference says reaction events only surface in the live watch stream. (docs/rpc.md:96, b61a3c95bcc3)
Real behavior proof and maintainer review: A collaborator independently reported focused cursor tests, a live Messages-compatible SQLite RPC fixture result, successful macOS build, and green macOS/Linux CI, while explicitly reserving the stable RPC contract for Peter’s approval. (Tests/imsgTests/RPCMessagesAfterTests.swift:1, b61a3c95bcc3)
Branch remains unmerged and unshipped: The requested external endpoint is not contained in current main; the latest release is v0.13.4 at the parent release commit, so no release provenance can establish this feature as already shipped. (Sources/imsg/RPCServer.swift:36, 1d8b679cc3a3)

Likely related people:

steipete: Peter Steinberger authored the current-main messagesAfter cursor implementation and explicitly stated that landing this new stable RPC contract should be gated on his approval. (role: existing behavior introducer and API decision owner; confidence: high; commits: 1d8b679cc3a3, d9bd9c754d50; files: Sources/IMsgCore/MessageStore+Messages.swift, Sources/imsg/RPCServer.swift, docs/json.md)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

Synchronize docs/json.md with the reaction behavior documented for messages.after.
Obtain explicit API-owner approval for the stable cursor and reaction-event contract.
Refresh the branch against current main and re-check mergeability after the contract decision.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (11 earlier review cycles; latest 8 shown)

reviewed 2026-08-01T11:54:28.593Z sha b61a3c9 :: needs maintainer review before merge. :: none
reviewed 2026-08-01T13:12:34.988Z sha b61a3c9 :: needs maintainer review before merge. :: none
reviewed 2026-08-02T00:16:14.244Z sha b61a3c9 :: needs maintainer review before merge. :: none
reviewed 2026-08-02T02:41:59.012Z sha b61a3c9 :: needs maintainer review before merge. :: none
reviewed 2026-08-02T05:08:23.663Z sha b61a3c9 :: found issues before merge. :: [P2] Align the public reaction-event documentation | [P3] Remove the release-owned changelog edit
reviewed 2026-08-02T07:14:35.076Z sha b61a3c9 :: found issues before merge. :: [P2] Align the shared reaction-event documentation | [P3] Remove the release-owned changelog entry
reviewed 2026-08-02T09:40:02.480Z sha b61a3c9 :: found issues before merge. :: [P2] Align the shared reaction-event documentation
reviewed 2026-08-02T12:47:42.730Z sha b61a3c9 :: found issues before merge. :: [P2] Align the shared reaction-event documentation

steipete · 2026-08-02T05:41:54Z

Independent verification on exact head b61a3c9 is technically clean. I recommend LAND, but this should remain gated on Peter’s approval of the new stable RPC contract rather than treated as routine maintenance.

Proof on macOS arm64:

Focused messages.after suite: 17 tests passed, including cursor progress across suppressed/coalesced URL-preview rows, cross-chat ordering, reaction exclusion, reply enrichment, attributed-body fallback, and empty advancing pages.
make build ARCHES="$(uname -m)": release CLI and universal helper built successfully.
Live release RPC integration against a Messages-compatible SQLite fixture: messages.after from ROWID 0 returned the real fixture message, next_rowid: 1, and has_more: false.
GitHub CI is green on macOS and Linux; the PR is cleanly mergeable.
Full-branch autoreview: clean, no actionable findings.

Before landing, maintainers should align docs/json.md with this endpoint’s reaction payload behavior and move/remove the contributor-authored changelog entry per repository policy. No contributor revision is requested; those are maintainer-side landing edits after the API decision.

Co-authored-by: Vincent Koc <vincentkoc@ieee.org>

feat(rpc): add rowid-forward message pagination

b61a3c9

clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 1, 2026

clawsweeper Bot mentioned this pull request Aug 1, 2026

feat(rpc): add rowid-forward message pagination #200

Closed

clawsweeper Bot added the P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. label Aug 1, 2026

clawsweeper Bot added the proof: sufficient Contributor real behavior proof is sufficient. label Aug 2, 2026

clawsweeper Bot mentioned this pull request Aug 2, 2026

fix(imessage): recover the oldest missed messages during catchup openclaw/openclaw#116961

Open

chore: refresh PR #201 for landing

f8474e4

Co-authored-by: Vincent Koc <vincentkoc@ieee.org>

steipete marked this pull request as ready for review August 2, 2026 18:39

steipete merged commit e9580a3 into main Aug 2, 2026
4 checks passed

Navigation Menu

Uh oh!

feat(rpc): add rowid-forward message pagination - #201

feat(rpc): add rowid-forward message pagination#201
steipete merged 2 commits into
mainfrom
feat/messages-after-rpc

vincentkoc commented Aug 1, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Aug 1, 2026 •

edited

Loading

Security

Review metrics

Root-cause cluster

Merge-risk options

Technical review

Labels

Evidence

Rank-up moves

Rating scale

Workflow

History

Uh oh!

steipete commented Aug 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

vincentkoc commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Contract

Validation

Uh oh!

clawsweeper Bot commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review

What this changes

Merge readiness

Review scores

Verification

How this fits together

Decision needed

Before merge

Findings

Security

Review metrics

Root-cause cluster

Merge-risk options

Technical review

Labels

Evidence

Rank-up moves

Rating scale

Workflow

History

Uh oh!

steipete commented Aug 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vincentkoc commented Aug 1, 2026 •

edited

Loading

clawsweeper Bot commented Aug 1, 2026 •

edited

Loading